Skip to content

Comments

feat: allow update of parent asset on UI#1957

Open
joshuaunity wants to merge 16 commits intomainfrom
feat/allow-update-of-parentasset-on-ui
Open

feat: allow update of parent asset on UI#1957
joshuaunity wants to merge 16 commits intomainfrom
feat/allow-update-of-parentasset-on-ui

Conversation

@joshuaunity
Copy link
Contributor

@joshuaunity joshuaunity commented Feb 6, 2026

Description

In this PR, I added a new field to the asset UI edit form, allowing users to update an asset's parent asset.

Look & Feel

Before
image

After
image

How to test

  1. Open up any asset properties page
  2. Navigate to the 'edit' op up modal on the left
  3. Find the new "parent asset" field and update
  4. Confirm it successfully updates

Further Improvements

In the future, it would be more powerful if we refactored these forms to use JavaScript for their update and other controls

Related Items

This PR closes #1441

Sign-off

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

@joshuaunity
feat: alloow update of parent asst on UI
c397dea 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity joshuaunity self-assigned this Feb 6, 2026
@joshuaunity joshuaunity added the enhancement New feature or request label Feb 6, 2026
@read-the-docs-community
Copy link

read-the-docs-community bot commented Feb 6, 2026 •
edited
Loading

Documentation build overview

📚 flexmeasures | 🛠️ Build #31527897 | 📁 Comparing d20dad4 against latest (6633ff2)


🔍 Preview build

Show files changed (19 files in total): 📝 19 modified | ➕ 0 added | ➖ 0 deleted
File Status
changelog.html 📝 modified
genindex.html 📝 modified
_autosummary/flexmeasures.api.common.responses.html 📝 modified
_autosummary/flexmeasures.api.v3_0.assets.html 📝 modified
_autosummary/flexmeasures.api.v3_0.sensors.html 📝 modified
_autosummary/flexmeasures.cli.utils.html 📝 modified
_autosummary/flexmeasures.data.models.data_sources.html 📝 modified
_autosummary/flexmeasures.data.models.forecasting.exceptions.html 📝 modified
_autosummary/flexmeasures.data.models.forecasting.html 📝 modified
_autosummary/flexmeasures.data.models.forecasting.pipelines.base.html 📝 modified
_autosummary/flexmeasures.data.models.forecasting.pipelines.train_predict.html 📝 modified
_autosummary/flexmeasures.data.schemas.forecasting.pipeline.html 📝 modified
_autosummary/flexmeasures.data.schemas.generic_assets.html 📝 modified
_autosummary/flexmeasures.data.schemas.utils.html 📝 modified
_autosummary/flexmeasures.data.services.data_sources.html 📝 modified
api/v3_0.html 📝 modified
cli/change_log.html 📝 modified
features/forecasting.html 📝 modified
tut/forecasting_scheduling.html 📝 modified

@joshuaunity joshuaunity requested a review from nhoening February 6, 2026 07:57
@Flix6x Flix6x changed the title feat: alloow update of parent asst on UI feat: allow update of parent asset on UI Feb 6, 2026
Flix6x
Flix6x reviewed Feb 9, 2026
View reviewed changes
joshuaunity and others added 9 commits February 9, 2026 10:56
@joshuaunity
refactor: add order to query for fetching all accounts assets to prop…
0dfca88 
…erties page

Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
Merge branch 'feat/allow-update-of-parentasset-on-ui' of github.com:F…
e6cb96d 
…lexMeasures/flexmeasures into feat/allow-update-of-parentasset-on-ui
@joshuaunity
fix: fixed issue where you cant update an asset properties if the use…
53d87c5 
… the same name of the asset itself

Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
fix: fixed validation logic with edgecase causing test to fail
ac916eb 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
fix: fixed another edge case with existing name validator
7dc4678 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
fix: final fix v1.0
eafa38a 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
fix: final fix v2.0
72a916e 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
Merge branch 'main' into feat/allow-update-of-parentasset-on-ui
9f87a5a
@joshuaunity
fix: final fix v2.1
5c140d2 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity joshuaunity requested a review from Flix6x February 12, 2026 11:02
nhoening
nhoening requested changes Feb 18, 2026
View reviewed changes
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it and it works nicely!

  1. One thing missing is a check server-side (which I stated in the issue).
  • The server-side code which processes the form should check if the parent asset really belongs to the same account.
  • Also make sure the asset is not listed as its own parent asset (this can be later extended to check for circularity, the parent asset should not be in the list of children or their descendants - needs a small recursive function).
  1. Also, do not include the asset itself in the list of options given to the template.

@joshuaunity
feat: remove current asset form parent list
e07e02f 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
Copy link
Contributor Author

joshuaunity commented Feb 19, 2026 •
edited
Loading

The server-side code which processes the form should check if the parent asset really belongs to the same account.

In the backend, I'm only sending assets in the same account, so the user doesn't even have the option to select a foreign asset

Also, make sure the asset is not listed as its own parent asset

This also doesn't happen anymore

Also, do not include the asset itself in the list of options given to the template.

Isn't this the same as the above

@nhoening

@nhoening
Copy link
Contributor

nhoening commented Feb 19, 2026

In the backend, I'm only sending assets in the same account, so the user doesn't even have the option to select a foreign asset

The backend needs to do its own checks and not trust the client.
Sure, that is more important with REST APIs, but this is not fundamentally different.
It's only one easy statement anyway

@joshuaunity
feat: validation for parent asset id while updating
49d5813 
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
@joshuaunity
Copy link
Contributor Author

joshuaunity commented Feb 20, 2026 •
edited
Loading

The server-side code which processes the form should check if the parent asset really belongs to the same account.

This is now implemented

How about the other things I replied to? Are we on the same page with that?

@nhoening

@nhoening
Copy link
Contributor

nhoening commented Feb 20, 2026

The server-side code which processes the form should check if the parent asset really belongs to the same account.

This is now implemented

How about the other things I replied to? Are we on the same page with that?

@nhoening

This is vague. Which things? I checked and I'm not sure what open reply I should look up.

@joshuaunity
Copy link
Contributor Author

joshuaunity commented Feb 20, 2026

The server-side code which processes the form should check if the parent asset really belongs to the same account.

This is now implemented
How about the other things I replied to? Are we on the same page with that?
@nhoening

This is vague. Which things? I checked and I'm not sure what open reply I should look up.

this part

image

@nhoening
Copy link
Contributor

nhoening commented Feb 20, 2026

This also doesn't happen anymore

If you say so :) I'll take a look in the review.

Isn't this the same as the above

I believe you are correct in this!

@joshuaunity joshuaunity requested a review from nhoening February 23, 2026 07:32
nhoening
nhoening approved these changes Feb 23, 2026
View reviewed changes
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks!

I approve, but if you could be so kind, add a comment and a docstring like I indicated, and I will merge this :)

flexmeasures/data/schemas/generic_assets.py
if parent_asset_id is not None:
parent_asset = db.session.get(GenericAsset, parent_asset_id)
if not parent_asset:
if parent_asset_id is None:
Copy link
Contributor

@nhoening nhoening Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added a second condition, so a short docstring would be nice:

"""
If a parent_asset_id is set, the parent asset should

  • exist
  • be in the same account of the context asset (if that is known via schema.context["asset"])

"""

flexmeasures/api/v3_0/assets.py
tags:
- Assets
"""
asset_data = request.get_json()
Copy link
Contributor

@nhoening nhoening Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just add a comment: "validating the path data ourselves, as we want to add context information (current asset).

Copy link
Contributor Author

@joshuaunity joshuaunity Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure, I can do that, I should be home in a few hours

@nhoening nhoening added this to the 0.31.0 milestone Feb 23, 2026
@nhoening
Copy link
Contributor

nhoening commented Feb 23, 2026

Oh, also you did not add a changelog entry yet @joshuaunity - please add these 3 things.

@joshuaunity
chore: final phase of changes
d20dad4 
Signed-off-by: Joshua Edward <oghenerobojosh01@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nhoening nhoening nhoening approved these changes

@Flix6x Flix6x Awaiting requested review from Flix6x

Assignees

@joshuaunity joshuaunity

Labels

enhancement New feature or request

Projects

None yet

Milestone

0.31.0

Development

Successfully merging this pull request may close these issues.

Edit asset: set parent ID

3 participants

@joshuaunity @nhoening @Flix6x